Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add keccak_256 hashing #578

Merged
merged 26 commits into from
Jan 22, 2025
Merged

Conversation

thil4n
Copy link
Contributor

@thil4n thil4n commented Jan 15, 2025

Purpose

This PR introduces implementation for keccak256 hashing in Ballerina Crypto Module. Keccak-256 is widely used in blockchain related technologies and currently Ballerina crypto module does not provide the Keccak-256 algorithm.

Examples

string dataString = "Hello Ballerina";
byte[] data = dataString.toBytes();
byte[] hash = crypto:hashKeccak256(data);

Checklist

  • Linked to an issue
  • Updated the changelog
  • Added tests
  • Updated the spec
  • Checked native-image compatibility

@MohamedSabthar
Copy link
Member

@thil4n let's update the changelog.md file and add your newly proposed API in this spec documentation: https://github.com/ballerina-platform/module-ballerina-crypto/blob/master/docs/spec/spec.md

Copy link
Member

@ThisaruGuruge ThisaruGuruge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks fine, let's update the spec and the changelog.

@daneshk
Copy link
Member

daneshk commented Jan 19, 2025

Code looks fine, let's update the spec and the changelog.

@thil4n In addition, Could you please add a proposal for this new API in the docs/proposals directory and create an issue for this? The example proposal can be found here

ThisaruGuruge
ThisaruGuruge previously approved these changes Jan 20, 2025
Copy link
Member

@ThisaruGuruge ThisaruGuruge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thil4n
Copy link
Contributor Author

thil4n commented Jan 20, 2025

Code looks fine, let's update the spec and the changelog.

@thil4n In addition, Could you please add a proposal for this new API in the docs/proposals directory and create an issue for this? The example proposal can be found here

Added a proposal, Also updated spec.md and changlog.md

@thil4n thil4n requested a review from MohamedSabthar January 20, 2025 07:39
@MohamedSabthar
Copy link
Member

@thil4n let's resolve the conflicts

@thil4n
Copy link
Contributor Author

thil4n commented Jan 20, 2025

@thil4n let's resolve the conflicts

Okay, will do.

@thil4n thil4n requested a review from MohamedSabthar January 21, 2025 04:36
ThisaruGuruge
ThisaruGuruge previously approved these changes Jan 21, 2025
Copy link
Member

@ThisaruGuruge ThisaruGuruge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

changelog.md Outdated Show resolved Hide resolved
@ThisaruGuruge ThisaruGuruge merged commit 00591dc into ballerina-platform:master Jan 22, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants